Reviewed 15 of 23 files at r1, all commit messages.
Reviewable status: 15 of 23 files reviewed, 26 unresolved discussions (waiting on @ychoquet)

packages/geoview-core/src/api/config/types/config-constants.ts line 28 at r1 (raw file):

 * Indeed, GeoCore is not an official Abstract Geoview Layer, but it can be used in schema types.
 */
export type TypeGeoviewLayerTypeWithGeoCore = TypeGeoviewLayerType | typeof CV_CONST_SUB_LAYER_TYPES.GEOCORE;

You cant remove the geocore from type and keep the same name ...TypeWithGeocore

packages/geoview-core/src/api/config/types/config-constants.ts line 155 at r1 (raw file):

        enableRotation: true,
        rotation: 0,
        maxExtent: [-125, 30, -60, 89],

The extent will depend of the projection and it is not max extent but only extent

"initialView": {
        "extent": [-90, 45, -65, 70]
      },

Do not forget to test the demo pages to see if your modification does what it is expected to do

packages/geoview-core/src/api/config/types/config-constants.ts line 172 at r1 (raw file):

    extraOptions: {},
  },
  theme: 'dark',

Default theme is geo.ca

packages/geoview-core/src/api/config/types/config-constants.ts line 186 at r1 (raw file):

  externalPackages: [],
  serviceUrls: {
    geocoreUrl: CV_CONFIG_GEOCORE_URL,

Add constant for geolocator and proxy url (value is in a comment below)

packages/geoview-core/src/api/config/types/config-validation-schema.json line 88 at r1 (raw file):

      "additionalProperties": false,
      "enum": ["dark", "light", "geo.ca"],
      "default": "dark"

default is geo.ca

packages/geoview-core/src/api/config/types/config-validation-schema.json line 245 at r1 (raw file):

      "properties": {
        "name": {
          "description": "External Package name. The name must be ideintical to the window external package object to load.",

Typo identical

packages/geoview-core/src/api/config/types/config-validation-schema.json line 276 at r1 (raw file):

        },
        "proxyUrl": {
          "description": "An optional proxy to be used for dealing with same-origin issues.  URL must either be a relative path on the same server or an absolute path on a server which sets CORS headers.",

default "https://maps.canada.ca/wmsproxy/ws/wmsproxy/executeFromProxy"

packages/geoview-core/src/api/config/types/config-validation-schema.json line 280 at r1 (raw file):

        },
        "geolocator": {
          "description": "Service end point to access geo location of searched value.",

default: "https://geolocator.api.geo.ca?keys=geonames,nominatim,locate"

packages/geoview-core/src/api/config/types/config-validation-schema.json line 284 at r1 (raw file):

        }
      },
      "required": ["geocoreUrl"]

Add the geolocator and proxy as required for the internal one

packages/geoview-core/src/api/config/types/config-validation-schema.json line 415 at r1 (raw file):

            "type": "number"
          },
          "default": [-125, 30, -60, 89]

Do not forget there 2 default, one for LCC and one for WM

packages/geoview-core/src/api/config/types/config-validation-schema.json line 627 at r1 (raw file):

          "minimum": 0,
          "maximum": 28,
          "default": 3.5

I see 3.5 here but when we run the code it is 4.5 (as define in your constant file and below)

packages/geoview-core/src/api/config/types/config-validation-schema.json line 799 at r1 (raw file):

          "properties": {
            "source": {
              "$ref": "#/definitions/TypeSourceImageEsriInitialConfig"

should be DynamicEsri.... because ImageEsri.... will be for image server. Below it is EsriFeature... here ImageEsri, We should have the same order... EsriFeatureInitial... and EsriDynamicInitial...

packages/geoview-core/src/api/config/types/config-validation-schema.json line 1061 at r1 (raw file):

          "type": "object",
          "properties": {
            "maxRecordCount": {

There is no maxRecord on image source (raster)

packages/geoview-core/src/api/config/types/config-validation-schema.json line 1094 at r1 (raw file):

      "allOf": [
        {
          "$ref": "#/definitions/TypeBaseVectorSourceInitialConfig"

There is duplication here.... all vector source should same base vector properties except format. transparent is not part of vector source

packages/geoview-core/src/api/config/types/config-validation-schema.json line 1271 at r1 (raw file):

      }
    },
    "TypeBaseVectorConfig": {

is it geometry? If so we should add to type name

packages/geoview-core/src/api/config/types/map-schema-types.ts line 12 at r1 (raw file):

import { EsriFeatureLayerEntryConfig } from '@config/types/classes/sub-layer-config/vector-leaf/esri-feature-layer-entry-config';

export { MapFeatureConfig } from '@config/types/classes/map-feature-config';

Why the export at the top?

packages/geoview-core/src/api/config/types/map-schema-types-new.ts line 1 at r1 (raw file):

import { AbstractGeoviewLayerConfig } from '@config/types/classes/geoview-config/abstract-geoview-layer-config';

Keep the same name.... this file is use in core. Do not duplicate files for type. The actual one is working. Only the thing for listOfLayerEntryConfig can be in another file only if needed

packages/geoview-core/src/api/config/types/map-schema-types-new.ts line 108 at r1 (raw file):

   * or an absolute path on a server which sets CORS headers.
   */
  proxyUrl?: string;

ike my other comment, optional for the user, mandatory for the internal... same for geolocator

packages/geoview-core/src/api/config/types/map-schema-types-new.ts line 153 at r1 (raw file):

/** Definition of the view settings. */
export type TypeViewSettings = {
  /** Settings for the initial view for map, default is zoomAndCenter of [3.5, [-90, 65]] */

you changed it for 4.5

packages/geoview-core/src/api/config/types/map-schema-types-new.ts line 163 at r1 (raw file):

  rotation?: number;
  /** The extent that constrains the view. Called with [minX, minY, maxX, maxY] extent coordinates.
   * Default [-125, 30, -60, 89].

Default is define from the projection

packages/geoview-core/src/api/config/types/map-schema-types-new.ts line 399 at r1 (raw file):

export interface TypeSourceImageEsriInitialConfig extends TypeBaseSourceInitialConfig {
  /** Maximum number of records to fetch (default: 0). */
  maxRecordCount: number;

No max on image source

packages/geoview-core/src/api/config/types/classes/map-feature-config.ts line 19 at r1 (raw file):

  TypeValidMapProjectionCodes,
  TypeValidVersions,
} from '@config/types/map-schema-types-new';

I do not like the new file here, preferable to use existing file and modify

packages/geoview-core/src/api/config/types/classes/geoview-config/vector-config/esri-feature-config.ts line 11 at r1 (raw file):

import { MapFeatureConfig } from '@/api/config/types/classes/map-feature-config';

export type TypeEsriFeatureLayerNode = GroupLayerEntryConfig | EsriFeatureLayerEntryConfig;

Why do you have a GroupLayerEntry type... A group is an abstraction, not a real type. When we are getting config for leaf you should not deal with Group. Group should be in its own config file I think. Esri feature and other vector will never have group not define in a config

For Dynamic we can but the metadata reading knows it right at the beginning if there is group or not and can create them prior to create the leaf. The whole structure can be done ahead of extracting info for leaf layer

packages/geoview-core/src/api/config/types/classes/geoview-config/vector-config/esri-feature-config.ts line 30 at r1 (raw file):

  constructor(layerConfig: TypeJsonObject, language: TypeDisplayLanguage, mapFeatureConfig?: MapFeatureConfig) {
    super(layerConfig, language, mapFeatureConfig);
    // this.geoviewLayerType = CV_CONST_LAYER_TYPES.ESRI_FEATURE;

Dead code

packages/geoview-core/src/api/config/types/classes/sub-layer-config/group-layer-entry-config.ts line 13 at r1 (raw file):

export class GroupLayerEntryConfig extends ConfigBaseClass {
  /** Used internally to distinguish layer groups derived from the metadata. */
  isMetadataLayerGroup = false;

Do we really need to make the difference?

packages/geoview-core/src/api/config/types/classes/sub-layer-config/raster-leaf/esri-dynamic-layer-entry-config.ts line 47 at r1 (raw file):

    super(layerConfig, initialSettings, language, geoviewLayerConfig, parentNode);
    // Set default values.
    if (!this.source) this.source = { maxRecordCount: 0, projection: 3978, format: 'png' };

No maxRecord on Dynamic

Reviewed 23 of 23 files at r1, all commit messages.
Reviewable status: all files reviewed, 27 unresolved discussions (waiting on @ychoquet)

packages/geoview-core/src/api/config/types/type-guards.ts line 10 at r1 (raw file):

import { CV_CONST_SUB_LAYER_TYPES, CV_CONST_LAYER_TYPES } from '@config/types/config-constants';
import { TypeJsonObject } from '@config/types/config-types';
import {

As a general note, type guards are, most of the time, unnecessary, as TypeScript automatically narrows down the type when performing conditional operations.

If we feel like we really need to, we can keep those custom type guards, but I'd probably just place them next to their Type definition (like where TypeBaseVectorConfig is defined). Having a specific file (which would serve for a bunch of type guards?) seems like overload.

Reviewable status: all files reviewed, 26 unresolved discussions (waiting on @Alex-NRCan and @jolevesq)

packages/geoview-core/src/api/config/types/config-constants.ts line 28 at r1 (raw file):

This is not a constant. Also, this type does not exists in the new configuration

packages/geoview-core/src/api/config/types/config-constants.ts line 155 at r1 (raw file):

The value here are used only if the user doesn't specify a projection code.

packages/geoview-core/src/api/config/types/config-constants.ts line 172 at r1 (raw file):

Done.

packages/geoview-core/src/api/config/types/config-constants.ts line 186 at r1 (raw file):

Done.

packages/geoview-core/src/api/config/types/config-validation-schema.json line 88 at r1 (raw file):

Done.

packages/geoview-core/src/api/config/types/config-validation-schema.json line 245 at r1 (raw file):

Done.

packages/geoview-core/src/api/config/types/config-validation-schema.json line 276 at r1 (raw file):

Done.

packages/geoview-core/src/api/config/types/config-validation-schema.json line 280 at r1 (raw file):

Done.

packages/geoview-core/src/api/config/types/config-validation-schema.json line 284 at r1 (raw file):

The internal config validation doesn't have to make these properties as required because if the user doesn't provide them, the default values will be assigned to them. The question is: do we have to set geocoreUrl as required for the input schema?

packages/geoview-core/src/api/config/types/config-validation-schema.json line 415 at r1 (raw file):

The default specified here is used only if the user doesn't specify a projection code

packages/geoview-core/src/api/config/types/config-validation-schema.json line 627 at r1 (raw file):

Done. I've changed the value in config-constants to 4.5

packages/geoview-core/src/api/config/types/config-validation-schema.json line 799 at r1 (raw file):

Done. Used TypeSourceEsriFeatureInitialConfig and TypeSourceEsriDynamicInitialConfig

packages/geoview-core/src/api/config/types/config-validation-schema.json line 1061 at r1 (raw file):

Are you sure? I ask because ersiDynamic layers can call getAllFeatureInfo as we can do with esriFeature layers.

packages/geoview-core/src/api/config/types/config-validation-schema.json line 1094 at r1 (raw file):

Done. TypeSourceEsriFeatureInitialConfig narrow down the format's domain (TypeVectorSourceFormats) defined in the parent TypeBaseVectorSourceInitialConfig to EsirJSON. This is normal and all other vector layers will do the same. I removed the transparent property from the TypeSourceEsriFeatureInitialConfig definition.

packages/geoview-core/src/api/config/types/config-validation-schema.json line 1271 at r1 (raw file):

Done. Renamed TypeBaseVectorConfig to TypeBaseVectorGeometryConfig.

packages/geoview-core/src/api/config/types/map-schema-types.ts line 12 at r1 (raw file):

The rule says all types defined in config validation schema MUST BE DEFINED in map schema types. That's why.

packages/geoview-core/src/api/config/types/map-schema-types-new.ts line 1 at r1 (raw file):

Done. I deleted the old map schema types file and replaced it with this new one.

packages/geoview-core/src/api/config/types/map-schema-types-new.ts line 108 at r1 (raw file):

Same question: Default is used if the user doesn't provide a value. This is the reason why the internal schema doesn't have to enforce a required constraint for these URLs. Also, input schema shouldn't have a required constraint on geocoreUrl since there is a default value.

packages/geoview-core/src/api/config/types/map-schema-types-new.ts line 153 at r1 (raw file):

Done.

packages/geoview-core/src/api/config/types/map-schema-types-new.ts line 163 at r1 (raw file):

Same reason as above, the default is used when the user doesn't provide a value at all.

packages/geoview-core/src/api/config/types/map-schema-types-new.ts line 399 at r1 (raw file):

Same question as above.

packages/geoview-core/src/api/config/types/type-guards.ts line 10 at r1 (raw file):

Type guards is used when we want to switch from the base type to the child type. Child of a base class doesn't have all the same additional properties. Using type guards not only does a type cast, it also verify that the cast is valid.

packages/geoview-core/src/api/config/types/classes/map-feature-config.ts line 19 at r1 (raw file):

Done. This was temporary. The file name is back to what it was before.

packages/geoview-core/src/api/config/types/classes/geoview-config/vector-config/esri-feature-config.ts line 11 at r1 (raw file):

I do not agree with the interpretation you make here. All geoview layer has in the listOfLayerEntryConfig a tree structure that can have a height of 1 or more depending on the user's need. All intermediary nodes must be a GroupLayerEntryConfig object and all leaves must be objects of a type corresponding to the geoview layer in use. In the present context, EsriFeatureLayerConfig instances can only contain EsriFeatureLayerEntryConfig.

packages/geoview-core/src/api/config/types/classes/geoview-config/vector-config/esri-feature-config.ts line 30 at r1 (raw file):

Done. Removed.

packages/geoview-core/src/api/config/types/classes/sub-layer-config/group-layer-entry-config.ts line 13 at r1 (raw file):

Yes, all nodes of the tree share a set of common properties.

packages/geoview-core/src/api/config/types/classes/sub-layer-config/raster-leaf/esri-dynamic-layer-entry-config.ts line 47 at r1 (raw file):

Same question as before.

Reviewed 39 of 43 files at r2, all commit messages.
Reviewable status: 43 of 47 files reviewed, 8 unresolved discussions (waiting on @Alex-NRCan and @ychoquet)

packages/geoview-core/src/api/config/types/config-validation-schema.json line 284 at r1 (raw file):

No , it can be only for the internal... geocore url is the same as the other. Only needed for the internal

packages/geoview-core/src/api/config/types/config-validation-schema.json line 1061 at r1 (raw file):

We may have to do some testing, but the max record is use to query feature to display on the map.... Do we have an ESRI with thousand of features and max record?

Ok, on le garde pour Dynamic...

packages/geoview-core/src/api/config/types/classes/geoview-config/vector-config/esri-feature-config.ts line 11 at r1 (raw file):

Yeah but the esri-config cannot be a group it must be esri-feature config. Then the group is group-config.ts

packages/geoview-core/src/api/config/types/classes/sub-layer-config/raster-leaf/esri-dynamic-layer-entry-config.ts line 47 at r1 (raw file):

Ok, we keep

packages/geoview-core/src/core/stores/store-interface-and-intial-values/ui-state.ts line 108 at r2 (raw file):

        get().uiState.setterActions.setActiveFooterBarTab(id);
      },
      setActiveAppBarTabId: (id: string) => {

Why have you changed this state file?

packages/geoview-core/src/core/utils/config/config-validation.ts line 10 at r2 (raw file):

import defaultsDeep from 'lodash/defaultsDeep';

import { TypeDisplayLanguage, TypeLocalizedString } from '@/api/config/types/map-schema-types';

We can keep this @api/config as it is the same path as @config but I do not understand why you revert all the @config you have done

Reviewable status: 43 of 47 files reviewed, 8 unresolved discussions (waiting on @Alex-NRCan, @jolevesq, and @ychoquet)

packages/geoview-core/src/api/config/types/config-validation-schema.json line 284 at r1 (raw file):

Done.

packages/geoview-core/src/api/config/types/config-validation-schema.json line 1061 at r1 (raw file):

Ok, we'll see. Let's keep it for the moment.

packages/geoview-core/src/api/config/types/classes/geoview-config/vector-config/esri-feature-config.ts line 11 at r1 (raw file):

Do not confuse the GeoView layer in the listOfGeoviewLayerConfig with the tree structure in the listOfLayerEntryConfig. The input schema defines the listOfLayerEntryConfig as a tree with intermediate nodes of GroupLayerEntryConfig and leaf nodes like EsriFeatureLayerEntryConfig in this case. In addition, all the types that can be part of the listOfLayerEntryConfig have a ...EntryConfig termination, and apart from the trunk nodes of type GroupLayerEntryConfig , a GeoView layer can only contain one leaf type (EsriFeatureLayerEntryConfig in this case). The internal schema follows the same rules.

packages/geoview-core/src/core/stores/store-interface-and-intial-values/ui-state.ts line 108 at r2 (raw file):

To resolve a collision duriong the rebase.

packages/geoview-core/src/core/utils/config/config-validation.ts line 10 at r2 (raw file):

I did not revert them. I think the rebase did that.

Reviewed 1 of 43 files at r2, 38 of 38 files at r3, 8 of 8 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Alex-NRCan)

Reviewed 5 of 5 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Alex-NRCan)

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Alex-NRCan)

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Alex-NRCan)